Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement Allocator for ! #82143

Closed
wants to merge 1 commit into from
Closed

Implement Allocator for ! #82143

wants to merge 1 commit into from

Conversation

petertodd
Copy link
Contributor

Useful to represent situations where something will never actually allocate, such as immutable version of a copy-on-write type.

I know there's a lot of traits where ! impls would make sense, and there's pros and cons of adding them. But this one has the advantage of being unstable, so adding it could help get more experience as to those pros and cons. Besides, I personally have an actual use-case for this impl in one of my projects. :)

@petertodd
Implement Allocator for !
fd6548e 
Useful to represent situations where something will never actually
allocate, such as immutable version of a copy-on-write type.
@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 15, 2021
@cuviper
Copy link
Member

cuviper commented Feb 16, 2021

Besides, I personally have an actual use-case for this impl in one of my projects. :)

Do you really need ! for that? How about your own empty enum as an uninhabited Allocator? You can use match *self {} as a point of divergence in the impl, so you don't need !'s coercion in the return value.

@jyn514 jyn514 added needs-fcp This change is insta-stable, so needs a completed FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 16, 2021
@petertodd
Copy link
Contributor Author

Besides, I personally have an actual use-case for this impl in one of my projects. :)

Do you really need ! for that? How about your own empty enum as an uninhabited Allocator? You can use match *self {} as a point of divergence in the impl, so you don't need !'s coercion in the return value.

Well, it's a stylistic question really. Right now the ! type docs suggest implementing it wherever makes sense: "When writing your own traits, ! should have an impl whenever there is an obvious impl which doesn't panic!." going on to discuss impl Trait. In my case, that's effectively what is happening: I have methods that would either return an Allocator, or something else.

Now, maybe the distinction to be made here is that you use Allocator as a "noun" trait - a type of thing - rather than a "verb" trait - the ability to do a thing.

@petertodd
Copy link
Contributor Author

Never mind, this is blocked by #81270 anyway.

@petertodd petertodd closed this Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@joshtriplett joshtriplett

Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@petertodd @rust-highfive @cuviper @joshtriplett @jyn514